Skip to content

Ungated fixes extracted from harmony-one/harmony#5122 - #16

Open
Frozen wants to merge 2 commits into
mainfrom
critical-fixes
Open

Ungated fixes extracted from harmony-one/harmony#5122#16
Frozen wants to merge 2 commits into
mainfrom
critical-fixes

Conversation

@Frozen

@Frozen Frozen commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

This PR extracts the 14 fixes marked Gated: No from harmony-one#5122.

These changes do not depend on StrictStateValidationEpoch, do not change its activation epochs, and do not include any of the fork-gated behavior from harmony-one#5122. They are intended to be safe to deploy without a protocol activation while remaining consensus-compatible with the current network.

Included fixes

Severity Area Fix
Critical Consensus / view change Reject VIEWCHANGE senders that are not committee participants.
Critical Slashing Skip external debt apportionment when total external stake is zero.
Critical Vote power Guard votepower.Compute when TotalEffectiveStake is zero.
Critical Cross-shard receipts Reject mismatched ShardIDs/CXShardHashes lengths and nil merkle proofs.
High Cross-shard receipts Bound pending receipt header epochs to current + 1 and cap requeues.
High Block storage Copy CommitSig and CommitBitmap into the correct fields.
High Rewards Bounds-check AddReward against the delegation snapshot.
Medium Staking / validator Apply MaxBLSPerValidator before BLS proof verification.
Low Block processing Check that the parent block exists before reading its root.
Low Cross-shard receipts Handle a nil CXReceiptsProof receiver.
Low Staking / validator Prevent slot-key slice aliasing on validator creation.
Low Node messages Guard empty block-message payloads.
Low Rewards Remove the unreachable unsigned lower-bound comparison.
Low Blockchain config Return a copy from GetVMConfig.

Explicitly excluded

All 14 rows marked Gated: Yes in harmony-one#5122 are excluded, including the gated EVM, delegation, reward, vote-power, receipt-import, and validator-state behavior changes. This PR also contains no StrictStateValidationEpoch configuration change.

Provenance and scope

Testing

Regression coverage is included for every behavioral fix. Relevant packages include:

  • consensus
  • consensus/votepower
  • core
  • core/state
  • core/types
  • node/harmony
  • staking/slash
  • staking/types

@Frozen
Ungated fixes from cross-shard, staking, rewards and consensus review
221e820 
Fork-free subset of harmony-one#5122: every fix here changes no
epoch-gated behaviour, so nodes running it stay in consensus with the
current network.

Critical:
- consensus: reject VIEWCHANGE senders not in the committee; a non-member
  signature left the M3 aggregate unverifiable against its bitmap
- slash: skip external debt apportionment when total external stake is
  zero (divide by zero on fully undelegated delegators)
- votepower: guard zero TotalEffectiveStake in Compute (divide by zero)
- core: reject CXReceiptsProof with mismatched ShardIDs/CXShardHashes
  lengths or a nil merkle proof

High:
- consensus: bound pending receipt header epoch to current+1 and cap
  requeues of proofs whose shard state never arrives
- types: CXReceiptsProof.Copy wrote CommitSig into CommitBitmap
- state: bounds-check AddReward against the delegation snapshot

Medium:
- staking: apply MaxBLSPerValidator before BLS proof verification

Low:
- core: nil parent check in MayBalanceMigration; nil receiver in
  ContainsEmptyField; GetVMConfig returns a copy; empty payload guard in
  HandleNodeMessage; unreachable uint64 comparison removed; slot key
  slice aliasing fixed
@greptile-apps

greptile-apps Bot commented Aug 19, 2026 •
edited
Loading

Copy link
Copy Markdown

Greptile Summary

This PR extracts 14 ungated defensive and correctness fixes without changing protocol activation epochs.

  • Adds validation for view-change membership, cross-shard receipt proofs, reward snapshots, validator slot keys, parent lookup, and empty node messages.
  • Bounds pending cross-shard receipt epochs and retries.
  • Corrects receipt-proof copying and isolates returned VM configuration.

Confidence Score: 4/5

The PR does not appear safe to merge while validators that synchronize across the retained height can continue without the required emergency recovery ViewID floor.

The previously reported recovery-floor failure remains: startup below the retained height leaves the floor at zero, and no post-synchronization path installs it after the node crosses that height.

Files Needing Attention: consensus/recovery_view_id.go and cmd/harmony/main.go

Important Files Changed
Filename Overview
consensus/checks.go Rejects view-change votes from senders outside the active committee.
consensus/consensus_block_proposing.go Bounds pending receipt epochs and limits retries when source shard state is unavailable.
core/block_validator.go Rejects missing merkle proofs and undersized shard-hash arrays before indexing them.
core/blockchain_impl.go Validates incoming receipt structure and returns an isolated VM configuration copy.
core/state/statedb.go Prevents reward distribution from indexing beyond the current delegation list.
core/state_processor.go Returns a descriptive error when a migration header's parent block is unavailable.
core/types/cx_receipt.go Correctly copies commit bitmaps and safely handles nil receipt-proof receivers.
staking/types/validator.go Copies slot-key storage and rejects excessive key counts before proof verification.

Reviews (2): Last reviewed commit: "fix: preserve v1.2 consensus compatibili..." | Re-trigger Greptile

greptile-apps[bot]
greptile-apps Bot reviewed Aug 19, 2026
View reviewed changes
Comment thread consensus/recovery_view_id.go
Comment on lines +87 to +89
if headHeight < retainedBlock {
return 0, false, nil
}

@greptile-apps greptile-apps Bot Aug 19, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Recovery floor remains unset

When a mainnet shard-0 or shard-1 validator starts below the retained height and later synchronizes across it, this startup-only check leaves its recovery floor at zero. The validator then uses ordinary ViewIDs and sends messages that recovery-aware peers reject, preventing it from participating in consensus.

@Frozen Frozen changed the title Critical fixes Ungated fixes from #5122 Aug 19, 2026
@Frozen Frozen changed the title Ungated fixes from #5122 Ungated fixes extracted from harmony-one/harmony#5122 Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Frozen